Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Earn battle points from trainer battles (with a variable) #5286

Closed
wants to merge 5 commits into from
Closed

Earn battle points from trainer battles (with a variable) #5286

wants to merge 5 commits into from

Conversation

laserXdolphin
Copy link

This allows the earning of Battle Points instead of money for scripted trainer battles by setting two variables.

  1. setvar VAR_0x8003, 1 // enable battle points routine and battle frontier transitions if the config is set to true
  2. setvar VAR_0x8004, 5 // the amount of BP that should be earned from this battle

Description

Mainly implemented the tutorial Scyrous made with slight changes:

  1. Used version 1.9.1 of the expansion (different BATTLESTRINGS_COUNT and Cmd_getmoneyreward).
  2. Added config for enabling battle frontier transition
  3. VAR_0x8003 and VAR_0x8004 are always reset in Cmd_getmoneyreward(void)

People who collaborated with me in this PR

This was originally written by by @Scyrous and can be found in the pret scripting tutorial section.
Original commit

Discord contact info

laserxdolphin

AlexOn1ine
AlexOn1ine reviewed Aug 29, 2024
View reviewed changes
src/battle_setup.c Outdated
Comment on lines 902 to 905
#if BATTLE_FRONTIER_INTRO_FOR_BP_FIGHTS
if (VarGet(gSpecialVar_0x8003) == 1)
return GetSpecialBattleTransition(FRONTIER_MODE_DOUBLES);
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the preproc block should be a normal if statement

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll gladly make the change but could you please tell me why that is the preferred way?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we can't verify if the code compiles with a preproc if it is set to false.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That totally makes sense, thank you for the explanation.

AlexOn1ine
AlexOn1ine reviewed Aug 30, 2024
View reviewed changes
src/battle_setup.c Outdated
@@ -899,6 +899,9 @@ u8 GetTrainerBattleTransition(void)
u32 trainerId = SanitizeTrainerId(gTrainerBattleOpponent_A);
u32 trainerClass = GetTrainerClassFromId(gTrainerBattleOpponent_A);

if (VarGet(gSpecialVar_0x8003) == 1 && BATTLE_FRONTIER_INTRO_FOR_BP_FIGHTS)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you switch those? The config should always be checked first

@AlexOn1ine AlexOn1ine added the new-feature Adds a feature label Aug 30, 2024
AlexOn1ine
AlexOn1ine reviewed Aug 30, 2024
View reviewed changes
Copy link
Collaborator

@AlexOn1ine AlexOn1ine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the easiest way to check Battle Points for review purposes?

src/battle_script_commands.c Outdated
Comment on lines 7771 to 7780
&& GetMonData(&gPlayerParty[i], MON_DATA_SPECIES_OR_EGG) != SPECIES_EGG)
&& GetMonData(&gPlayerParty[i], MON_DATA_SPECIES_OR_EGG) != SPECIES_EGG)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the alignment here was correct. Please revert.

Copy link
Author

@laserXdolphin laserXdolphin Aug 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say the easiest way to review it, is to toggle the Frontier Pass in the Debug Menu under Flags&Vars.
Which allows you to check your BP on your Trainer Card.
review

@laserXdolphin laserXdolphin marked this pull request as draft August 31, 2024 09:12
@laserXdolphin
Copy link
Author

laserXdolphin commented Aug 31, 2024 •
edited
Loading

I turned this into a draft because I found a bug where a trainer might drop bp.
I think it has something to do with repel reuse since gSpecialVar_0x8003 is set in sprays.c as well
gSpecialVar_0x8003 = count;
Possible solutions would be to use a number count can't reach for the activation condition or to use a different special var.
I have not found a way yet to reproduce this 100% but will post it once I find it.

@AlexOn1ine
Copy link
Collaborator

I turned this into a draft because I found a bug where a trainer might drop bp. I think it has something to do with repel reuse since gSpecialVar_0x8003 is set in sprays.c as well gSpecialVar_0x8003 = count; Possible solutions would be to use a number count can't reach for the activation condition or to use a different special var. I have not found a way yet to reproduce this 100% but will post it once I find it.

Using a different special variable would probably be best here. Just make sure to confirm that it does not clash with a different variable.

@ghoulslash
Copy link
Collaborator

This should not depend on scripting variables. Instead I would suggest adding a callnative or specia to set an ewram variable

@Scyrous
Copy link

Scyrous commented Sep 7, 2024

This should not depend on scripting variables. Instead I would suggest adding a callnative or specia to set an ewram variable

I concur. While I'm happy to see my commit is useful to some people, the implementation should be heavily scrutinized as it's not very good and quite limited in scope. That said, I have no desire to revisit the code myself anytime soon - I don't even remember what I did or why (thanks to a long hiatus).

@AlexOn1ine
Copy link
Collaborator

This should not depend on scripting variables. Instead I would suggest adding a callnative or specia to set an ewram variable

I concur. While I'm happy to see my commit is useful to some people, the implementation should be heavily scrutinized as it's not very good and quite limited in scope. That said, I have no desire to revisit the code myself anytime soon - I don't even remember what I did or why (thanks to a long hiatus).

I'm going to close the PR then

@AlexOn1ine AlexOn1ine closed this Sep 12, 2024
@laserXdolphin
Copy link
Author

Sounds good, sadly I currently don´t have the time to work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexOn1ine AlexOn1ine AlexOn1ine left review comments

Assignees
No one assigned
Labels
new-feature Adds a feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@laserXdolphin @AlexOn1ine @ghoulslash @Scyrous